Another option would be to consider the previous behavior where comments were stripped a bug, and and make this simple into bug fix without the extra option.

The PR description is missing the most important part -> the motivation. What's the use case? Why is 'preserveLeadingComments' needed? Why don't trailing comments need to be preserved? Or maybe comments in between? (I guess, a comment before a comment won't get parsed).

The SQL that is running is visible in observability tools. It's a common pattern to prepend a comment the query to make it visible in the observability tools (prepend vs append is important, because the full query is often not visible).

-- some explanation for why we need this alter
-- or why we think it's safe to run at all
ALTER something_complicated_and_risky;

The obvious problem is that given sequence query -> comment -> query, it's not obvious to which query the comment belongs.

SELECT 1; -- hello
SELECT 2; -- world

vs

-- hello
SELECT 1;

-- world 
SELECT 2;

This PR assumes that the second case is far more common and that it's not worth trying to fine tune the heuristics to support the first case as well. And as consequence the trailing comments are not supported, since there is no query to attach them to.

Or maybe comments in between

Comment in between is a comment before query. Multiple comments before a query are supported.

If we produce [sql, comment, sql, comment] it can be easily processed on the user side as [sql, comment+sql] - without any assumptions on the structure.

Isn't that a bit more universal? We may bundle such an iterator as well 🤔

That would be a major BC break. And you can't actually process this on user-side any better than this PR does it, unless we provide additional metadata such as line number and offset.

That should all be possible but it would be much larger in scope and require a new major version.

That would be a major BC break

Well, first we assess the arch & design, then the means to achieve BC, right?

E.g., bool $includeComments = false is pretty much the same way of change producing the mentioned output.

unless we provide additional metadata such as line number and offset

Could you give an example when this is required to parse it correctly then?

The problematic part on the user side is the detection if something is a comment, isn't it?

Could you give an example when this is required to parse it correctly then?

If you want to resolve to which query a comment belong, you need the positional information to determine if the comment started on the same line as the previous query. Or if it started on a new line.

The problematic part on the user side is the detection if something is a comment, isn't it?

Yes. That's why I assume it is BC break, because you need to wrap the strings with Query and Comment values classes.

Possible BC solution, but not pretty

	/**
	 * @param  positive-int $chunkSize
	 * @return Iterator<($includeComments is true ? Query|Comment : string)>
	 * @throws RuntimeException
	 */
	public function parseFile(
		string $path,
		int $chunkSize = self::DEFAULT_CHUNK_SIZE,
		bool $includeComments = false,
	): Iterator;

or

	/**
	 * @param  positive-int $chunkSize
	 * @return Iterator<Query|Comment>
	 * @throws RuntimeException
	 */
	public function parseFileWithComments(
		string $path,
		int $chunkSize = self::DEFAULT_CHUNK_SIZE,
	): Iterator;

I prefer the solution in this PR to both of these.

It seems you are focused on the API and BC, and not on the merit of what functionality we want to offer. So let me switch to this approach as well :)

My Kotlin-like mind would offer this.
Yes, it is only binary compatible (aka runtime compatible), not source-compatible.

https://phpstan.org/r/70b27277-20ff-446e-9790-22dbccc77392

/**
 * @template O
 */
interface Mapper {
	/**
	 * @param Iterator<int, Query|Comment> $iterator
	 * @return Iterator<int, O>
	 */
	function map(Iterator $iterator): Iterator;
}

/**
 * @template O
 * @implements IMultiQueryParser<O>
 */
class BaseMultiQueryParser implements IMultiQueryParser {
	/**
	 * @param Mapper<O> $mapper
	 */
	function __construct(
		private Mapper $mapper = new QueryOnlyMapper(),
	) {}

	/**
	 * @param  Iterator<string> $stream
	 * @return Iterator<O>
	 */
	abstract public function parseStringStream(Iterator $stream): Iterator;
}

EDIT: PHPStorm type inference sucks, tho.

Yes, basically, I'm trying to come up with a solution that won't hardcode the very custom non-standardized behavior attaching comments to the following queries.

Obviously, it makes very much sense for your company/use case. Yet, somebody may come and tell that the separator cannot be two empty lines, because then the meaning of the comment is not specific to the next query. Those POV are similarly valid as many another approchaes. That makes me feel I would abstract it differently.

If we provide custom inner callback / interface, then we don't need to change the final type from Iterable<string>, so that would I think solve everything.

Implemented as CommentStrategy interface.